Conversation
Walkthrough구매 주문 API의 모든 엔드포인트에 사용자 컨텍스트(UserContext) 파라미터를 추가하고, 관련 서비스 메서드 시그니처를 업데이트했습니다. 요청 DTO에서 요청자 필드를 제거하고, 서비스 구현에서 사용자 컨텍스트로부터 요청자 정보를 도출하는 방식으로 변경했습니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Controller
participant Service
participant Repo as Repository
Client->>Controller: GET /orders/pending<br/>(`@CurrentUser` UserContext)
activate Controller
Controller->>Service: getPendingOrders(user,<br/>startDate, endDate,<br/>searchKeyword, pageable)
activate Service
Note over Service: validUser(user)<br/>Derive requesterCode<br/>from region + workType
Service->>Service: Access control check<br/>(if 본사 required)
Service->>Repo: Query orders by<br/>requesterCode, userId,<br/>date range, etc.
activate Repo
Repo-->>Service: PageResponse
deactivate Repo
Service-->>Controller: PageResponse<br/>HeadPurchaseOrderResponse
deactivate Service
Controller-->>Client: JSON response
deactivate Controller
Estimated code review effort🎯 4 (복잡함) | ⏱️ ~45분 상세 검토 필요 영역:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/gearfirst/backend/api/order/controller/PurchaseOrderController.java(5 hunks)src/main/java/com/gearfirst/backend/api/order/dto/request/PurchaseOrderRequest.java(0 hunks)src/main/java/com/gearfirst/backend/api/order/service/PurchaseOrderService.java(1 hunks)src/main/java/com/gearfirst/backend/api/order/service/PurchaseOrderServiceImpl.java(14 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/gearfirst/backend/api/order/dto/request/PurchaseOrderRequest.java
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-28T05:05:49.715Z
Learnt from: ksoheee
Repo: Gear-First/gearfirst-order-BE PR: 26
File: src/main/java/com/gearfirst/backend/api/order/controller/PurchaseOrderController.java:93-98
Timestamp: 2025-10-28T05:05:49.715Z
Learning: In the Gear-First order backend (gearfirst-order-BE) Java Spring Boot project, when an endpoint performs state changes on the PurchaseOrder entity (such as calling order.completeRepair()), ensure the endpoint's Operation summary and description clearly document that state modification occurs, and use appropriate HTTP methods (POST/PATCH/PUT) rather than GET, even if the endpoint also returns data.
Applied to files:
src/main/java/com/gearfirst/backend/api/order/service/PurchaseOrderServiceImpl.javasrc/main/java/com/gearfirst/backend/api/order/controller/PurchaseOrderController.javasrc/main/java/com/gearfirst/backend/api/order/service/PurchaseOrderService.java
📚 Learning: 2025-10-30T09:27:59.696Z
Learnt from: ksoheee
Repo: Gear-First/gearfirst-order-BE PR: 28
File: src/main/java/com/gearfirst/backend/api/order/service/PurchaseOrderServiceImpl.java:146-148
Timestamp: 2025-10-30T09:27:59.696Z
Learning: In the gearfirst-order-BE project, the `completeRepairAndGetParts` method in `PurchaseOrderServiceImpl` is called exclusively from the repair reception page, ensuring that the `receiptNum` parameter is always present and does not require null validation.
Applied to files:
src/main/java/com/gearfirst/backend/api/order/service/PurchaseOrderServiceImpl.javasrc/main/java/com/gearfirst/backend/api/order/controller/PurchaseOrderController.javasrc/main/java/com/gearfirst/backend/api/order/service/PurchaseOrderService.java
Comment on lines
+49
to
+55
| Long userId = Long.parseLong(user.getUserId()); | ||
| String userName = user.getUsername(); | ||
| String role = user.getRank(); | ||
| String workType = user.getWorkType(); | ||
| String region = user.getRegion(); | ||
| String requesterCode = region + workType; | ||
|
|
There was a problem hiding this comment.
숫자가 아닌 userId에 대한 예외 처리 누락
게이트웨이가 내려주는 UserContext.userId는 외부 입력과 동일한 신뢰 수준인데, 지금은 Long.parseLong(user.getUserId())를 바로 호출하고 있어서 숫자가 아니면 NumberFormatException이 터져 5xx가 발생합니다. validUser에서도 숫자 여부 검증이 없어 UnAuthorizedException으로 처리되지 않습니다. 토큰 스펙이 바뀌거나 잘못된 토큰이 왔을 때 장애로 이어지니, validUser 단계에서 숫자 여부까지 확인하고 실패 시 인증 예외를 던지도록 보강해 주세요. 동일한 파싱이 반복되는 다른 메서드에도 영향을 주니 함께 정리 부탁드립니다.
private void validUser(UserContext user) {
- if (user == null || user.getUserId() == null) {
- throw new UnAuthorizedException(ErrorStatus.USER_UNAUTHORIZED.getMessage());
- }
+ if (user == null || user.getUserId() == null) {
+ throw new UnAuthorizedException(ErrorStatus.USER_UNAUTHORIZED.getMessage());
+ }
+ try {
+ Long.parseLong(user.getUserId());
+ } catch (NumberFormatException e) {
+ throw new UnAuthorizedException(ErrorStatus.USER_UNAUTHORIZED.getMessage());
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/main/java/com/gearfirst/backend/api/order/service/PurchaseOrderServiceImpl.java
around lines 49 to 55, the code directly calls Long.parseLong(user.getUserId())
which will throw NumberFormatException for non-numeric userId; update validation
so validUser verifies user.getUserId() is a valid numeric string and throws the
existing UnAuthorizedException on failure, replace direct parseLong calls with a
single utility method (e.g., parseUserIdOrThrow) used everywhere to centralize
parsing and avoid repeated try/catch, and refactor other methods that parse
userId to use this validation/parsing helper so malformed tokens result in a
controlled authentication exception instead of a 5xx error.
Contributor
Author
There was a problem hiding this comment.
게이트웨이에서 넘겨주는 userid는 String 형태잖아 그래서 Long.parseLong이렇게 바꾸는건데
There was a problem hiding this comment.
@ksoheee 네, String에서 Long으로 변환하는 부분은 이해했습니다! 제가 우려했던 부분은 만약 게이트웨이에서 숫자가 아닌 userId가 넘어올 경우 NumberFormatException이 발생해서 5xx 에러가 나는 상황이었습니다.
게이트웨이가 항상 유효한 숫자 형태의 userId를 보장한다면 추가 검증은 필요 없을 것 같습니다. 혹시 게이트웨이에서 토큰 파싱 시 userId의 형식을 검증하고 있나요? 만약 그렇다면 현재 구현으로 충분합니다. 👍
🧠 Learnings used
Learnt from: ksoheee
Repo: Gear-First/gearfirst-order-BE PR: 28
File: src/main/java/com/gearfirst/backend/api/order/service/PurchaseOrderServiceImpl.java:146-148
Timestamp: 2025-10-30T09:27:59.696Z
Learning: In the gearfirst-order-BE project, the `completeRepairAndGetParts` method in `PurchaseOrderServiceImpl` is called exclusively from the repair reception page, ensuring that the `receiptNum` parameter is always present and does not require null validation.
Learnt from: ksoheee
Repo: Gear-First/gearfirst-order-BE PR: 26
File: src/main/java/com/gearfirst/backend/api/order/controller/PurchaseOrderController.java:93-98
Timestamp: 2025-10-28T05:05:49.715Z
Learning: In the Gear-First order backend (gearfirst-order-BE) Java Spring Boot project, when an endpoint performs state changes on the PurchaseOrder entity (such as calling order.completeRepair()), ensure the endpoint's Operation summary and description clearly document that state modification occurs, and use appropriate HTTP methods (POST/PATCH/PUT) rather than GET, even if the endpoint also returns data.
Contributor
Author
There was a problem hiding this comment.
아니 게이트웨이에서는 userId를 항상 String으로 넘겨주지 jwt 토큰을 파싱해서 넘겨주는건데
There was a problem hiding this comment.
@ksoheee 아, 이해했습니다! 게이트웨이에서 JWT 토큰을 검증하고 파싱해서 넘겨주는 거군요. 그렇다면 게이트웨이 단에서 이미 토큰의 유효성을 확인하고 있으니, 여기서 넘어오는 userId는 신뢰할 수 있는 값이겠네요.
게이트웨이가 JWT 검증을 담당하고 있다면 현재 구현으로 충분합니다. 제 우려를 명확하게 설명해주셔서 감사합니다! 👍
🧠 Learnings used
Learnt from: ksoheee
Repo: Gear-First/gearfirst-order-BE PR: 28
File: src/main/java/com/gearfirst/backend/api/order/service/PurchaseOrderServiceImpl.java:146-148
Timestamp: 2025-10-30T09:27:59.696Z
Learning: In the gearfirst-order-BE project, the `completeRepairAndGetParts` method in `PurchaseOrderServiceImpl` is called exclusively from the repair reception page, ensuring that the `receiptNum` parameter is always present and does not require null validation.
Learnt from: ksoheee
Repo: Gear-First/gearfirst-order-BE PR: 26
File: src/main/java/com/gearfirst/backend/api/order/controller/PurchaseOrderController.java:93-98
Timestamp: 2025-10-28T05:05:49.715Z
Learning: In the Gear-First order backend (gearfirst-order-BE) Java Spring Boot project, when an endpoint performs state changes on the PurchaseOrder entity (such as calling order.completeRepair()), ensure the endpoint's Operation summary and description clearly document that state modification occurs, and use appropriate HTTP methods (POST/PATCH/PUT) rather than GET, even if the endpoint also returns data.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Summary
Summary by CodeRabbit
릴리스 노트
Bug Fixes
Refactor